[test] Limit the number of launched threads in shift_left_right.pass with the OMP backend to avoid timeouts
#1928
Conversation
…resolve timeouts Signed-off-by: Matthew Michel <[email protected]>
Signed-off-by: Matthew Michel <[email protected]>
mmichel11
requested review from
timmiesmith,
MikeDvorskiy,
dmitriy-sobolev and
danhoeflinger
There was a problem hiding this comment.
I don't love that this is necessary. In other words, The better fix is probably to improve our implementation to better chunk the data with a larger chunk size so that we can handle an appropriate number of threads to match the hardware. Or, if limiting the threads is truly the best option, then our implementation should probably control that internally.
If we see this issue with performance, then users would as well.
In the short term, I'm OK with making this change to stop the timeouts, if we also add a github issue to explore this further with a fix to the implementation, rather than the test.
Functionally, the code looks fine to me.
I agree this is our issue to fix and opened which #1929 uses |
We have recently seen timeouts in CI in
shift_left_right.passwhen running on CPUs with large core counts using the OMP backend. This has occurred mostly with debug builds but also in a small number of release builds. Due to the small problem sizes in our tests, the grain size per thread is too small when a large number of threads are launched resulting in significant performance loss due to excessive parallelism and cross-socket traffic.To resolve this issue, we cap the number of threads to
32inshift_left_right.passwhich enables the tests to pass in a reasonable time while also launching enough threads to assess the correctness of the parallel implementation.